Update HybridWebViewQueryStringHelper.cs#31597
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
jsuarezruiz
left a comment
There was a problem hiding this comment.
@Walkramis Could you rebase to fix the conflict?
049b128 to
b9acd69
Compare
@jsuarezruiz It is now updated with the latest edits/file move |
|
/azp run maui-pr-devicetests, maui-pr-uitests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Took a look at this — the fix is correct and worth merging. A few notes: Correctness
Suggested polish before merging
Out of scope but worth flagging The same bug exists in CI Last |
Fixes dotnet#31472. When loading a URL with a fragment (such as the 'index.html#code=...' redirect produced by MSAL/OAuth flows), the HybridWebView was treating the entire literal string as a file path and failing to find the local asset. Update RemovePossibleQueryString to also treat '#' as a delimiter so the fragment is dropped before the relative path is resolved. Caches the delimiter array as a static readonly field to avoid allocating on every call, and extends the existing unit test theory with fragment-bearing inputs (including a case where '#' precedes '?' and the exact URL shape from the issue report). Co-authored-by: Walkramis <16719812+Walkramis@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4f9f652 to
3e1afac
Compare
🤖 AI Summary
📊 Review Session —
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🧪 WebUtils_Tests WebUtils_Tests |
🛠️ BUILD ERROR | ✅ PASS — 17s |
🔴 Without fix — 🧪 WebUtils_Tests: 🛠️ BUILD ERROR · 22s
Determining projects to restore...
Restored /home/vsts/work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 1.32 sec).
Restored /home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj (in 4 sec).
Restored /home/vsts/work/1/s/src/Essentials/src/Essentials.csproj (in 3.9 sec).
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14031550
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14031550
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(285,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(295,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(302,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(312,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(319,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(326,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(338,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(348,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(360,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(371,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentials/test/UnitTests/Essentials.UnitTests.csproj]
🟢 With fix — 🧪 WebUtils_Tests: PASS ✅ · 17s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14031550
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14031550
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
Essentials.UnitTests -> /home/vsts/work/1/s/artifacts/bin/Essentials.UnitTests/Debug/net10.0/Microsoft.Maui.Essentials.UnitTests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Essentials.UnitTests/Debug/net10.0/Microsoft.Maui.Essentials.UnitTests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.01] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.14] Discovering: Microsoft.Maui.Essentials.UnitTests
[xUnit.net 00:00:00.60] Discovered: Microsoft.Maui.Essentials.UnitTests
[xUnit.net 00:00:00.62] Starting: Microsoft.Maui.Essentials.UnitTests
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: null, expected: "") [11 ms]
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: "https://0.0.0.1/index.html#code=1234asdf", expected: "https://0.0.0.1/index.html") [4 ms]
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: "https://example.com/path#frag?notquery", expected: "https://example.com/path") [< 1 ms]
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: "https://example.com/path?query=1#frag", expected: "https://example.com/path") [< 1 ms]
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: "https://example.com?foo=bar", expected: "https://example.com") [< 1 ms]
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: "https://example.com/index.html#code=abc", expected: "https://example.com/index.html") [< 1 ms]
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: "https://example.com", expected: "https://example.com") [< 1 ms]
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: "https://example.com/path?query=1&other=2", expected: "https://example.com/path") [< 1 ms]
Passed Tests.WebUtils_Tests.RemovePossibleQueryString_ReturnsExpected(input: "", expected: "") [< 1 ms]
[xUnit.net 00:00:00.84] Finished: Microsoft.Maui.Essentials.UnitTests
Passed Tests.WebUtils_Tests.ResolveRelativePath_ValidRelative_ReturnsPath [< 1 ms]
Passed Tests.WebUtils_Tests.ResolveRelativePath_EncodedDotDot_HandledCorrectly [11 ms]
Passed Tests.WebUtils_Tests.ResolveRelativePath_RootRequest_ReturnsEmpty [< 1 ms]
Passed Tests.WebUtils_Tests.ResolveRelativePath_DifferentOrigin_ReturnsNull [< 1 ms]
Passed Tests.WebUtils_Tests.ResolveRelativePath_DoubleSlash_MakeRelativeUri_ProducesRooted_ReturnsNull [< 1 ms]
Passed Tests.WebUtils_Tests.ResolveRelativePath_SubPath_ReturnsPath [< 1 ms]
Test Run Successful.
Total tests: 15
Passed: 15
Total time: 2.1234 Seconds
⚠️ Failure Details
- 🛠️ WebUtils_Tests without fix: build failed before tests could run
/home/vsts/work/1/s/src/Essentials/test/UnitTests/FileSystemUtils_Tests.cs(285,33): error CS0117: 'FileSystemUtils' does not contain a definition for 'EnsureFileName' [/home/vsts/work/1/s/src/Essentia...
📁 Fix files reverted (137 files)
.config/dotnet-tools.jsoneng/Signing.propseng/Version.Details.xmleng/Versions.propseng/pipelines/ci-copilot.ymlsrc/Controls/samples/Controls.Sample/Pages/Controls/MapsGalleries/MapPinsGallery.xamlsrc/Controls/samples/Controls.Sample/Pages/Controls/MapsGalleries/MapPinsGallery.xaml.cssrc/Controls/src/Build.Tasks/SetPropertiesVisitor.cssrc/Controls/src/Core/ActionSheetArguments.cssrc/Controls/src/Core/AlertArguments.cssrc/Controls/src/Core/BindableObject.cssrc/Controls/src/Core/BindableProperty.cssrc/Controls/src/Core/Button/Button.iOS.cssrc/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/Android/SearchHandlerAppearanceTracker.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFragmentContainer.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRenderer.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellRenderer.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellFlyoutLayoutManager.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRenderer.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRootRenderer.cssrc/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewController.cssrc/Controls/src/Core/Editor/Editor.Mapper.cssrc/Controls/src/Core/Editor/Editor.iOS.cssrc/Controls/src/Core/Entry/Entry.Mapper.cssrc/Controls/src/Core/Entry/Entry.iOS.cssrc/Controls/src/Core/Handlers/Items/Android/GridLayoutSpanSizeLookup.cssrc/Controls/src/Core/Handlers/Items/Android/MauiCarouselRecyclerView.cssrc/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cssrc/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cssrc/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cssrc/Controls/src/Core/Handlers/Items2/iOS/CarouselViewController2.cssrc/Controls/src/Core/Handlers/Items2/iOS/GroupableItemsViewController2.cssrc/Controls/src/Core/Handlers/Items2/iOS/ItemsViewController2.cssrc/Controls/src/Core/Handlers/Items2/iOS/ItemsViewDelegator2.cssrc/Controls/src/Core/Handlers/Items2/iOS/StructuredItemsViewController2.cssrc/Controls/src/Core/Handlers/Shell/Windows/ShellView.cssrc/Controls/src/Core/Hosting/AppHostBuilderExtensions.cssrc/Controls/src/Core/Label/Label.Mapper.cssrc/Controls/src/Core/Label/Label.iOS.cssrc/Controls/src/Core/NavigationPage/NavigationPage.cssrc/Controls/src/Core/Platform/AlertManager/AlertManager.Android.cssrc/Controls/src/Core/Platform/AlertManager/AlertManager.cssrc/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cssrc/Controls/src/Core/Platform/Android/DragAndDropGestureHandler.cssrc/Controls/src/Core/Platform/Android/TabbedPageManager.cssrc/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.iOS.cssrc/Controls/src/Core/Platform/iOS/Extensions/FormattedStringExtensions.cssrc/Controls/src/Core/Platform/iOS/Extensions/LabelExtensions.cssrc/Controls/src/Core/PromptArguments.cssrc/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txtsrc/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txtsrc/Controls/src/Core/RadioButton/RadioButton.cssrc/Controls/src/Core/RadioButton/RadioButtonGroup.cssrc/Controls/src/Core/RadioButton/RadioButtonGroupController.cssrc/Controls/src/Core/Setter.cssrc/Controls/src/Core/Shadow.cssrc/Controls/src/Core/Shapes/Shape.cssrc/Controls/src/Core/Shell/Shell.cssrc/Controls/src/Core/Shell/ShellNavigationManager.cssrc/Controls/src/Core/TabbedPage/TabbedPage.Windows.cssrc/Controls/src/Core/VisualElement/VisualElement.cssrc/Controls/src/Core/VisualStateManager.cssrc/Controls/src/Xaml/ApplyPropertiesVisitor.cssrc/Controls/src/Xaml/MarkupExtensions/OnIdiomExtension.cssrc/Controls/src/Xaml/MarkupExtensions/StaticResourceExtension.cssrc/Core/maps/src/Handlers/Map/MapHandler.Android.cssrc/Core/maps/src/Handlers/MapPin/MapPinHandler.Android.cssrc/Core/maps/src/PublicAPI/net-android/PublicAPI.Unshipped.txtsrc/Core/src/Graphics/MauiDrawable.Android.cssrc/Core/src/Handlers/Button/ButtonHandler.Android.cssrc/Core/src/Handlers/Button/ButtonHandler.cssrc/Core/src/Handlers/Button/ButtonHandler.iOS.cssrc/Core/src/Handlers/DatePicker/DatePickerHandler.MacCatalyst.cssrc/Core/src/Handlers/Editor/EditorHandler.iOS.cssrc/Core/src/Handlers/Entry/EntryHandler.cssrc/Core/src/Handlers/Entry/EntryHandler.iOS.cssrc/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Standard.cssrc/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Tizen.cssrc/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Windows.cssrc/Core/src/Handlers/HybridWebView/HybridWebViewHandler.cssrc/Core/src/Handlers/Label/LabelHandler.cssrc/Core/src/Handlers/Label/LabelHandler.iOS.cssrc/Core/src/Handlers/RadioButton/RadioButtonHandler.cssrc/Core/src/Handlers/RadioButton/RadioButtonHandler.iOS.cssrc/Core/src/Handlers/SearchBar/SearchBarHandler.iOS.cssrc/Core/src/Handlers/ShapeView/ShapeViewHandler.Standard.cssrc/Core/src/Handlers/ShapeView/ShapeViewHandler.cssrc/Core/src/Handlers/Switch/SwitchHandler.iOS.cssrc/Core/src/Handlers/TimePicker/TimePickerHandler.Android.cssrc/Core/src/Handlers/TimePicker/TimePickerHandler.Windows.cssrc/Core/src/Handlers/TimePicker/TimePickerHandler.cssrc/Core/src/Handlers/TimePicker/TimePickerHandler.iOS.cssrc/Core/src/Hosting/EssentialsMauiAppBuilderExtensions.cssrc/Core/src/Platform/Android/ContainerView.cssrc/Core/src/Platform/Android/MauiSwipeView.cssrc/Core/src/Platform/Android/MauiWebView.cssrc/Core/src/Platform/Android/TimePickerExtensions.cssrc/Core/src/Platform/Windows/ContentPanel.cssrc/Core/src/Platform/Windows/MauiPasswordTextBox.cssrc/Core/src/Platform/Windows/MauiToolbar.xaml.cssrc/Core/src/Platform/Windows/RootNavigationView.cssrc/Core/src/Platform/Windows/ScrollViewerExtensions.cssrc/Core/src/Platform/Windows/TimePickerExtensions.cssrc/Core/src/Platform/iOS/ButtonExtensions.cssrc/Core/src/Platform/iOS/LayerExtensions.cssrc/Core/src/Platform/iOS/MauiPageControl.cssrc/Core/src/Platform/iOS/MauiSwipeView.cssrc/Core/src/Platform/iOS/MauiTextView.cssrc/Core/src/Platform/iOS/MauiView.cssrc/Core/src/Platform/iOS/TimePickerExtensions.cssrc/Core/src/Platform/iOS/WrapperView.cssrc/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txtsrc/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txtsrc/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txtsrc/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txtsrc/Core/src/ViewExtensions.cssrc/Core/src/WindowExtensions.cssrc/Essentials/src/AssemblyInfo/AssemblyInfo.shared.cssrc/Essentials/src/Clipboard/Clipboard.shared.cssrc/Essentials/src/FileSystem/FileSystemUtils.android.cssrc/Essentials/src/FileSystem/FileSystemUtils.shared.cssrc/Essentials/src/MainThread/MainThread.netstandard.cssrc/Essentials/src/MainThread/MainThread.shared.cssrc/Essentials/src/MediaPicker/MediaPicker.ios.cssrc/Essentials/src/Types/Shared/WebUtils.shared.cssrc/Graphics/src/Graphics/Platforms/Android/PlatformGraphicsView.cssrc/Graphics/src/Graphics/Platforms/MaciOS/PlatformCanvas.cssrc/Graphics/src/Graphics/Platforms/Windows/PlatformGraphicsView.cssrc/Graphics/src/Graphics/Platforms/iOS/PlatformGraphicsView.cs
New files (not reverted):
src/Controls/src/Core/Handlers/Items/iOS/IScrollTrackingDelegator.cssrc/Controls/src/Core/Platform/AlertManager/DelegateAlertSubscription.cssrc/Core/src/Handlers/HybridWebView/HybridWebViewHelper.cssrc/Core/src/ScreenshotDispatch.cs
🧪 UI Tests — Category Detection
Detected UI test categories: Essentials
🔍 Pre-Flight — Context & Validation
Issue: #31472 - HybridWebViewQueryStringHelper.RemovePossibleQueryString removes '?' but not other special characters e.g. '#'
PR: #31597 - Update HybridWebViewQueryStringHelper.cs
Platforms Affected: Android, iOS, Windows (all HybridWebView platforms)
Files Changed: 1 implementation (WebUtils.shared.cs), 1 test (WebUtils_Tests.cs)
Key Findings
WebUtils.RemovePossibleQueryStringwas only stripping?query strings, not#fragment identifiers- OAuth/OIDC implicit flow redirects URLs like
https://0.0.0.0/index.html#code=abc, causing HybridWebView to look up a file namedindex.html#code=abcwhich does not exist - The fix extends the method to strip both
?and#usingIndexOfAnywith astatic readonly char[]field - All three HybridWebView callers (Android
MauiHybridWebViewClient, iOSHybridWebViewHandler.iOS, WindowsHybridWebViewHandler.Windows) are covered via the shared utility - BlazorWebView
QueryStringHelper.cshas the same bug but is mirrored from ASP.NET Core upstream — out of scope for this PR - CLA signature pending from contributor; CI pipelines just triggered (May 6, 2026)
Code Review Summary
Verdict: LGTM
Confidence: high
Errors: 0 | Warnings: 0 | Suggestions: 0
Key code review findings:
- No errors, warnings, or actionable suggestions found. The existing
@kubafloreview comment already captured all nits (method rename as follow-up, BlazorWebView parity as separate issue). All nits were addressed in this PR (static readonly field, 4 new unit tests).
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31597 | Use IndexOfAny(new[]{'?','#'}) with static readonly field in WebUtils.RemovePossibleQueryString |
✅ PASSED (Gate) | WebUtils.shared.cs, WebUtils_Tests.cs |
Original PR |
🔬 Code Review — Deep Analysis
Code Review — PR #31597
Independent Assessment
What this changes: WebUtils.RemovePossibleQueryString is extended to strip URL fragments (the #… portion) in addition to query strings (?…). A static readonly char[] field replaces the inline IndexOf('?') call, and IndexOfAny is used to find whichever delimiter appears first. Four new unit test cases cover the fragment-stripping scenarios.
Inferred motivation: A HybridWebView app using OAuth/OIDC (MSAL implicit flow) navigates to a redirect URL like https://0.0.0.0/index.html#code=abc. The WebView interceptor must strip the fragment before looking up the local file asset; without this fix it tries to open a file named index.html#code=abc… which does not exist, causing a 404 or blank page.
Reconciliation with PR Narrative
Author claims: "Change so both # and ? will be taken into consideration to remove to allow to load the appropriate local file." Links issue #31472.
Agreement: Complete. The code implements precisely the workaround the author self-identified in the issue, extended cleanly to all three HybridWebView platforms via the shared utility method.
Findings
All meaningful code-level observations were already captured by @kubaflo (MEMBER) in their May 6 comment:
- Method naming (
RemovePossibleQueryStringnow strips fragments too) — noted, acknowledged as a follow-up rename. - BlazorWebView
QueryStringHelper.csparity gap — noted, explicitly marked out of scope, suggested as a separate issue. static readonly char[]field — already done in this PR.- Test coverage — already added in this PR.
No new inline findings surfaced beyond those.
Correctness Deep-Dive
All three HybridWebView callers are covered:
MauiHybridWebViewClient.cs:71(Android) ✓HybridWebViewHandler.iOS.cs:239(iOS) ✓HybridWebViewHandler.Windows.cs:167(Windows) ✓
All three call RemovePossibleQueryString → new Uri(result) → AppOriginUri.IsBaseOf(uri) → ResolveRelativePath. Stripping # before this chain is exactly correct.
IndexOfAny(char[]) semantics are correct: Inherently ordinal (char equality), which is the right behavior for ? and # ASCII delimiters.
Percent-encoded %23: IndexOfAny searches the raw string, so a literal %23 in a path component is not mistakenly stripped. Correct.
Test case "https://example.com/path#frag?notquery" → "https://example.com/path": Validates that IndexOfAny returns the first occurrence (position of #), not the position of ?. This is the key correctness invariant.
CI Status
Status:
license/cla— queued (not yet completed); CLA signature pending from contributorBuild Analysis— in progress- Device/UI test pipelines triggered May 6, 2026 — results not yet visible
CI is not red due to the PR's code changes.
Devil's Advocate
"Could stripping # cause a regression for apps that DON'T use fragments?"
IndexOfAny returns -1 if neither ? nor # is found, so the code path for plain https://0.0.0.0/index.html is unchanged. ✓
"What if a Blazor app hits the same OAuth fragment scenario?"
QueryStringHelper.cs (BlazorWebView) still only strips ?. That is a pre-existing gap not introduced by this PR. Not a regression. ✓
"Is there any concern about Substring performance?"
This method is called inside a WebView URL-interception callback (cold path). Allocation is irrelevant here. ✓
Verdict: LGTM
Confidence: high
Summary: The fix is minimal, correct, and well-tested. It addresses a real user-visible bug (OAuth fragment redirects breaking HybridWebView) by extending the existing RemovePossibleQueryString utility to also strip #-fragments. All three HybridWebView platform handlers are covered via the shared method. The two nits worth tracking (method rename, BlazorWebView parity) are already documented in existing review comments and appropriate as follow-up items.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 (claude-opus-4.6) | Uri.TryCreate + uri.Query + uri.Fragment suffix; span IndexOfAny fallback |
✅ PASS | 1 file | Framework URI parsing; more complex (+15 lines) |
| 2 | try-fix-2 (claude-sonnet-4.6) | url.AsSpan().IndexOfAny('?','#') — allocation-free .NET 6+ two-char span scan |
✅ PASS | 1 file | Simplest: 1-line change, no static field, no allocation |
| 3 | try-fix-3 (gpt-5.3-codex) | Uri.GetLeftPart(UriPartial.Path) |
1 file | Baseline revert caused pre-existing compile error (unrelated to fix) | |
| 4 | try-fix-4 (gpt-5.5) | static readonly Regex("[?#].*", Compiled) + .Replace(url, "") |
✅ PASS | 1 file | Compiled Regex; correct but heavier than needed |
| PR | PR #31597 | IndexOfAny(s_queryOrFragmentDelimiters) with static readonly char[] |
✅ PASSED (Gate) | 2 files | Original PR — clean, explicit, readable |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | Yes | url.Split('?','#')[0] — allocating variation, not materially different |
| claude-sonnet-4.6 | 2 | No | NO NEW IDEAS |
| gpt-5.3-codex | 2 | Yes | Two separate IndexOf calls with min-pick — functionally identical to IndexOfAny |
Exhausted: Yes — no materially new approaches remain
Selected Fix: PR's fix — minimal, explicit, readable, already has tests and Gate passed
Comparison Notes
- try-fix-2 (
url.AsSpan().IndexOfAny('?','#')) is arguably marginally simpler (1 line vs 3 lines in PR) but the PR's approach withstatic readonly char[]is more explicit about intent and matches codebase patterns - All three passing candidates (PR, tf-1, tf-2, tf-4) produce identical outputs for all test cases
- The PR fix is the best candidate: already reviewed, tested, comes with 4 new unit tests, and follows a clear intent-revealing pattern
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #31472, HybridWebView fragment stripping |
| Code Review | LGTM (high) | 0 errors, 0 warnings, 0 suggestions |
| Gate | ✅ PASSED | android — tests fail without fix, pass with fix |
| Try-Fix | ✅ COMPLETE | 4 attempts (3 pass, 1 blocked), cross-pollination exhausted |
| Report | ✅ COMPLETE |
Code Review Impact on Try-Fix
Code review returned LGTM with zero findings, so no ❌ Error hints were provided to try-fix models. The hints field described the code-review verdict and directed each model to explore a different alternative strategy. Models chose: URI framework parsing (tf-1), allocation-free span scan (tf-2), URI path extraction (tf-3, blocked by env), and compiled Regex (tf-4). All passing candidates produced functionally identical outputs.
Summary
PR #31597 fixes a real user-visible bug in WebUtils.RemovePossibleQueryString — the method previously stripped only ? query strings, not # fragments. This caused HybridWebView to attempt loading non-existent files (e.g. index.html#code=abc) when OAuth/OIDC redirects included fragment identifiers. The fix is minimal (1 implementation file, 1 test file), correct, and well-tested. All three HybridWebView platform callers (Android, iOS, Windows) are covered via the shared utility.
Root Cause
HybridWebView's request interceptor calls WebUtils.RemovePossibleQueryString to get a clean file path from the intercepted URL. The method used IndexOf('?') only, missing the URL fragment delimiter #. RFC 3986 defines # as starting the fragment, which must be stripped before looking up a local asset. The fix uses IndexOfAny(new[]{'?','#'}) with a static readonly field, correctly stripping whichever delimiter appears first.
Fix Quality
Winner: pr (the PR's original fix)
The PR's fix is clean, minimal, and follows established codebase patterns. Four independent try-fix explorations found no superior alternative:
| Candidate | Approach | Result | vs. PR |
|---|---|---|---|
pr |
IndexOfAny(char[]) with static readonly field |
✅ Gate passed | Baseline |
pr-plus-reviewer |
Same as PR (no reviewer corrections needed) | ✅ Gate passed | Identical |
try-fix-1 |
Uri.TryCreate + fragment suffix computation |
✅ PASS | More complex (+15 lines), no advantage |
try-fix-2 |
url.AsSpan().IndexOfAny('?','#') |
✅ PASS | 1-line, marginally simpler, but loses explicit delimiter documentation |
try-fix-3 |
Uri.GetLeftPart(UriPartial.Path) |
Environment issue; structurally heavier | |
try-fix-4 |
Compiled Regex("[?#].*") |
✅ PASS | Unnecessary overhead for a simple delimiter scan |
The PR's pattern (static readonly char[] + IndexOfAny) is explicit about which delimiters are handled, self-documenting through the field name s_queryOrFragmentDelimiters, and consistent with the codebase's existing pattern in ParseQueryString. The span approach (tf-2) would be a valid micro-optimization but the overhead is irrelevant for a cold URL interception path.
Non-code blockers to merge:
- CLA signature pending from
@Walkramis(bot request posted May 6, 2026) - CI pipelines triggered May 6, 2026 — final results pending
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
| static class WebUtils | ||
| { | ||
| #if !NETSTANDARD | ||
| static readonly char[] s_queryOrFragmentDelimiters = new[] { '?', '#' }; |
There was a problem hiding this comment.
[moderate] Cross-Platform Behavioral Consistency — QueryStringHelper.RemovePossibleQueryString in src/BlazorWebView/src/SharedSource/QueryStringHelper.cs is a parallel implementation of this same method used by all BlazorWebView handlers (Android WebKitWebViewClient.cs:119, iOS BlazorWebViewHandler.iOS.cs:318, Windows WinUIWebViewManager.cs:89, Tizen BlazorWebViewHandler.Tizen.cs:125). It still only strips ? and will fail for the same fragment-only URL scenario (e.g., MSAL OAuth redirect https://host/index.html#code=1234asdf). This PR fixes HybridWebView but leaves BlazorWebView broken under the identical scenario. The SharedSource/ origin (ASP.NET Core upstream copy) is noted, but the behavioral gap should be addressed — either by updating QueryStringHelper here (accepting upstream drift) or by opening a tracked issue before merging.
kubaflo
left a comment
There was a problem hiding this comment.
Hi! Could you please accept the policy?
<!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change `WebUtils.RemovePossibleQueryString` only stripped query strings (`?`) but not URL fragments (`#`). This caused HybridWebView to fail loading local files when the URL contained a fragment identifier (e.g. `index.html#code=abc`). The fix uses `IndexOfAny` to find the first `?` or `#` delimiter and strips everything from that point forward. Supersedes #31597. ### Issues Fixed Fixes #31472 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
<!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change `WebUtils.RemovePossibleQueryString` only stripped query strings (`?`) but not URL fragments (`#`). This caused HybridWebView to fail loading local files when the URL contained a fragment identifier (e.g. `index.html#code=abc`). The fix uses `IndexOfAny` to find the first `?` or `#` delimiter and strips everything from that point forward. Supersedes #31597. ### Issues Fixed Fixes #31472 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
<!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change `WebUtils.RemovePossibleQueryString` only stripped query strings (`?`) but not URL fragments (`#`). This caused HybridWebView to fail loading local files when the URL contained a fragment identifier (e.g. `index.html#code=abc`). The fix uses `IndexOfAny` to find the first `?` or `#` delimiter and strips everything from that point forward. Supersedes #31597. ### Issues Fixed Fixes #31472 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description of Change
Change so both # and ? will be taken into consideration to remove to allow to load the appropriate local file
Issues Fixed
Issue as described in this issue report
#31472